fix(tests): flush-observation waits assert on timeout + raise CI deadlines - #331
Merged
Conversation
… deadlines Follow-up to #330. Its own CI run still failed test_flush_thread_appends_ ops_to_shard_stores at the filter-bucket assert: the wait_until(...) guarding that disk read discarded its return value, so a deadline miss on a slow shared runner fell through to `filter_store.read()` returning None and panicked with the generic "filter bucket should exist after insert" — which can't be told apart from a real write failure. - Every disk-observation wait_until now asserts its bool with a timeout- specific message, so a deadline miss says "timed out waiting for ..." instead of masquerading as a missing shard. (Two sites — filter and sort — previously discarded the return entirely.) - Raise disk-observation deadlines 15s -> 60s. CI runners are far slower than a 32-core dev box (the suite takes 5+ min there); these are correctness waits, not latency contracts, and wait_until returns the instant the condition holds, so a generous ceiling is free on healthy runs. Verified: flush_thread 15/15 isolated; full lib suite 2/2 clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #330 (merged as 5dc7c2e), whose own CI run still failed
test_flush_thread_appends_ops_to_shard_storesatconcurrent_engine.rs:15102.Root cause of the residual flake
The
wait_until(...)guarding the filter-bucket disk read discarded its return value. On a slow shared CI runner the 15s deadline was missed, so control fell through tofilter_store.read()(which returnedNone) and the next line panicked with the genericfilter bucket should exist after insert. That message can't be distinguished from a genuine write failure — and it wasn't the sweep race (#330's fix held).Fixes
wait_untilnow asserts its bool with a timeout-specific message (timed out waiting for ...), so a deadline miss announces itself instead of masquerading as a missing shard. Two sites (filter, sort) previously discarded the return entirely; the other two folded it into a compound assert with a misleading message.wait_untilreturns the instant the condition holds, so a generous ceiling is free on healthy runs.No production code touched (this is test-only; the sweep tolerance from #330 already covers the ShardStore race).
Verification
test_flush_thread_appends_ops_to_shard_stores: 15/15 isolated.--skip test_min_tracked_value_after_expansion): 2/2 clean.🤖 Generated with Claude Code